Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implement policy resources for lb monitor profile #1036

Merged
merged 1 commit into from
Jan 11, 2024
Merged

Implement policy resources for lb monitor profile #1036

merged 1 commit into from
Jan 11, 2024

Conversation

2ez4szliu
Copy link
Contributor

Implement several lb monitor profile resources using policy API including:

  • lb_http_monitor
  • lb_https_monitor
  • lb_icmp_monitor
  • lb_passive_monitor
  • lb_tcp_monitor
  • lb_udp_monitor

@2ez4szliu 2ez4szliu force-pushed the lb-monitors branch 7 times, most recently from 2759fb8 to e527a57 Compare November 21, 2023 04:03
@2ez4szliu 2ez4szliu linked an issue Nov 21, 2023 that may be closed by this pull request
resource for lb policy monitor #578
Closed
salv-orlando
salv-orlando reviewed Nov 23, 2023
View reviewed changes
nsxt/policy_utils.go
@@ -402,3 +402,38 @@ func getElemOrEmptyMapFromMap(d map[string]interface{}, key string) map[string]i
}
return make(map[string]interface{})
}

func setPolicyLbHTTPHeaderInSchema(d *schema.ResourceData, attrName string, headers []model.LbHttpRequestHeader) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not mind instead creating a generic function that sets an attribute which is a list of key/value pairs. This can be used for tags, headers, and likely more.

I do not have however a strong preference.

nsxt/resource_nsxt_policy_lb_icmp_monitor_profile.go Outdated
"data_length": {
Type: schema.TypeInt,
Optional: true,
Description: "The data size (in byte) of the ICMP healthcheck packet format: int64",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it matter to Terraform users that format is int64?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for all in64 parameters from SDK we convert them to Terraform TypeInt

nsxt/resource_nsxt_policy_lb_icmp_monitor_profile.go
displayName := d.Get("display_name").(string)
description := d.Get("description").(string)
tags := getPolicyTagsFromSchema(d)
dataLength := int64(d.Get("data_length").(int))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why the SDK used int64 for these parameters.... wasted bits imho

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems SDK use int64 for all int parameters...
I noticed that in the SDK, the comment next to some parameters says that this parameter is of format int32, however in the SDK code the parameter is still defined as an int64.
For example, Revision

// The _revision property describes the current revision of the resource. To prevent clients from overwriting each other's changes, PUT operations must include the current _revision of the resource, which clients should obtain by issuing a GET operation. If the _revision provided in a PUT request is missing or stale, the operation will be rejected. format: int32
Revision *int64

website/docs/r/policy_lb_http_monitor_profile.html.markdown Outdated Show resolved Hide resolved
nsxt/resource_nsxt_policy_lb_https_monitor_profile.go Outdated Show resolved Hide resolved
website/docs/r/policy_lb_http_monitor_profile.html.markdown Outdated Show resolved Hide resolved
website/docs/r/policy_lb_icmp_monitor_profile.html.markdown Outdated Show resolved Hide resolved
@2ez4szliu 2ez4szliu force-pushed the lb-monitors branch 2 times, most recently from 2a59e10 to b39579e Compare December 2, 2023 04:37
annakhm
annakhm reviewed Dec 5, 2023
View reviewed changes
nsxt/lb_utils.go Outdated Show resolved Hide resolved
@2ez4szliu
Copy link
Contributor Author

/test-all

@2ez4szliu 2ez4szliu force-pushed the lb-monitors branch 3 times, most recently from ca48c53 to a8dcc5f Compare January 5, 2024 01:04
@2ez4szliu
Copy link
Contributor Author

/test-all

@2ez4szliu 2ez4szliu force-pushed the lb-monitors branch 4 times, most recently from bfa65e9 to 56ee5f5 Compare January 8, 2024 18:16
@2ez4szliu
Copy link
Contributor Author

/test-all

@2ez4szliu 2ez4szliu force-pushed the lb-monitors branch 4 times, most recently from 71fe3d3 to 88103cc Compare January 9, 2024 02:47
@2ez4szliu
Copy link
Contributor Author

/test-all

@2ez4szliu
Copy link
Contributor Author

/test-all

@2ez4szliu
Copy link
Contributor Author

/test-all

Implement policy resources for lb monitor profile
b11f7a0 
Implement LB monitor resources using policy API.

Types of LB monitors include:
- lb_http_monitor
- lb_https_monitor
- lb_icmp_monitor
- lb_passive_monitor
- lb_tcp_monitor
- lb_udp_monitor

Signed-off-by: Shizhao Liu <[email protected]>
@2ez4szliu
Copy link
Contributor Author

/test-all

@2ez4szliu 2ez4szliu merged commit 664d914 into vmware:master Jan 11, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@salv-orlando salv-orlando salv-orlando left review comments

@annakhm annakhm annakhm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

resource for lb policy monitor
3 participants
@2ez4szliu @salv-orlando @annakhm